Fix BL0012 false positive on await foreach and await using - #68720
Fix BL0012 false positive on await foreach and await using#68720AzeemullahRg wants to merge 1 commit into
Conversation
|
Thanks for your PR, @AzeemullahRg. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
a4adc00 to
362a078
Compare
StateHasChangedAnalyzer located the first and last await by collecting
AwaitExpressionSyntax nodes. `await foreach`, `await using (...) { }` and
`await using var x = ...;` carry their await as a keyword token on the statement, so a
method whose only awaits take one of those forms looked await-free and every
StateHasChanged in it was reported as redundant.
Track the region the method can suspend in instead, taking the statement span for
`await foreach` and for the block form of `await using`, and the enclosing block for a
using declaration, whose disposal runs when that block exits. Calls outside that region
are still reported, so behaviour for ordinary awaits is unchanged.
Found while validating dotnet#68484.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018irsMDStKQdmB1k7fkUmqM
362a078 to
ef44591
Compare
|
@dotnet-policy-service agree |
CI triage: both failures are pre-existing flakes, unrelated to this changeThis PR only touches 1. Build Analysis matched this to Known Build Error #68708 (same "top rendered index was -1" assertion). There is also an open quarantine request for this exact test: #68724. 2. A SQLite connection torn down under a concurrent test, not a product failure. Build Analysis reports a 0.84% historical failure rate for this test. It is not matched to a Known Build Error issue, so it shows as unmatched. 1 failure out of 85,417 tests in that build. Ask: could a maintainer re-run the two failed legs? I don't have permission to re-queue dnceng-public builds. Happy to push an empty commit instead if that's preferred. |
Problem
BL0012reportsStateHasChangedcalls that are required, whenever the enclosing method's only awaits areawait foreachorawait using.The call is what paints each item as it arrives. Accepting the offered code fix removes it, and the list then stays empty until the stream completes and appears in one batch.
Cause
StateHasChangedAnalyzerlocated the first and last await by collectingAwaitExpressionSyntaxnodes:Three forms do not produce an
AwaitExpressionSyntax. They carry their await as a keyword token on the statement instead:await foreach (...)CommonForEachStatementSyntax.AwaitKeywordawait using (...) { }UsingStatementSyntax.AwaitKeywordawait using var x = ...;LocalDeclarationStatementSyntax.AwaitKeywordA method whose only awaits take one of those forms therefore looked await-free. The analyzer took its
Count == 0branch, which treats every call in the method as redundant on the grounds thatComponentBaserenders once the method returns.Fix
Track the region the method can suspend in, rather than a set of await positions. Each construct contributes the span over which it can yield:
await exprawait foreach (...)await using (...) { }await using var x = ...;The reporting rule is unchanged: a call is redundant when it sits outside that region, so behaviour for ordinary awaits stays as it was.
The last row is why a call after an
await usingdeclaration is no longer reported. Its disposal runs at the end of the enclosing block, so the call still has an await after it andComponentBasehas not rendered yet.Tests
Five added to
StateHasChangedAnalyzerTest:await foreach, inside anawait usingblock, and after anawait usingdeclarationawait foreachis still reportedWith the analyzer change reverted the three fail and the two pass.
Found while validating #68484.